Fix Contact list modal broken in landscape mode - #96751
Conversation
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d72affc684
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Good for product.
@GCyganek Do you know how to create Submit workspace? It looks like it is a beta feature in OD, right? |
@GCyganek Thanks for your suggestion. I am going to use it to test the change in this PR. |
|
@GCyganek I'm unable to scroll the list without dismissing the keyboard. As soon as I start scrolling, the keyboard is automatically closed. Screen.Recording.2026-07-26.at.19.21.06.mov |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
is there a case when this is possible? I thought this is the default behaviour, see this is how it works on main when I test different popovers with input + selection list on both ios and Android: Screen.Recording.2026-07-27.at.09.39.08.movScreen.Recording.2026-07-27.at.11.03.19.mov |
@GCyganek If it is general behavior, we can skip it. |
|
Now there is only one comment needs to be address #96751 (comment) |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-07-29.at.09.45.33.movScreen.Recording.2026-07-29.at.09.46.07.movAndroid: mWeb ChromeiOS: HybridAppScreen.Recording.2026-07-29.at.09.41.08.movScreen.Recording.2026-07-29.at.09.42.30.moviOS: mWeb SafariMacOS: Chrome / Safari |
| return availableWindowHeight - keyboardActiveHeight; | ||
| } | ||
|
|
||
| return windowHeight * CONST.MODAL_MAX_HEIGHT_TO_WINDOW_HEIGHT_RATIO_LANDSCAPE_MODE; |
There was a problem hiding this comment.
Should we use availableWindowHeight instead of windowHeight?
| const listStyle = StyleUtils.getEmojiPickerListHeight( | ||
| isListFiltered, | ||
| windowHeight * (isInLandscapeMode ? CONST.MODAL_MAX_HEIGHT_TO_WINDOW_HEIGHT_RATIO_LANDSCAPE_MODE : 0.95) - keyboardHeight, | ||
| isInLandscapeMode ? calculateModalHeightInLandscapeMode(windowHeight, topSafeAreaInset, keyboardActiveHeight) : (windowHeight - topSafeAreaInset) * 0.95 - keyboardActiveHeight, |
There was a problem hiding this comment.
Portrait-mode height formula changed as a side-effect
The old code was windowHeight * 0.95 - keyboardHeight. The new code subtracts topSafeAreaInset before scaling and switches from keyboardHeight to keyboardActiveHeight — both are behavioral changes in portrait mode. Was the portrait-mode change intentional here, or should the non-landscape branch stay closer to the original?





Explanation of Change
Fixes Contact list modal which is broken in landscape mode, also adjusts height when the keyboard is open. Adds a shared
calculateModalHeightInLandscapeModeutil (accounts for top safe-area inset and active keyboard height) and uses it in the Submit To popover and Emoji Picker (where there was a similar logic for modal height). Also hides the confirm button while the keyboard is active in landscape so that the input and list is visible while typing with open keyboard, and simplifies the landscape checks to no longer requireisSmallScreenWidth.Fixed Issues
$ #95096
PROPOSAL: N/A
Tests
Prerequisites: use Submit workspace
Offline tests
N/A
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Screen.Recording.2026-07-22.at.16.23.08.mov
Android: mWeb Chrome
Screen.Recording.2026-07-22.at.16.35.41.mov
iOS: Native
Screen.Recording.2026-07-22.at.16.24.41.mov
iOS: mWeb Safari
Screen.Recording.2026-07-22.at.16.37.51.mov
MacOS: Chrome / Safari
Screen.Recording.2026-07-22.at.16.29.10.mov